Have the frontend and new swift-driver look in an external -sdk for non-Darwin platform runtime libraries and modules too
#79621
Conversation
|
@swift-ci please test |
|
swiftlang/swift-driver#1822 |
1 similar comment
|
swiftlang/swift-driver#1822 |
lib/Frontend/CompilerInvocation.cpp
Outdated
| FrontendOpts.UseSharedResourceFolder ? "swift" : "swift_static", | ||
| getPlatformNameForTriple(Triple)); | ||
| // Check for eg <sdkRoot>/usr/lib/swift/linux/ | ||
| if (llvm::sys::fs::exists(SDKResourcePath)) { |
There was a problem hiding this comment.
I initially tried simply checking if <sdkRoot>/usr/lib/swift/, ie a Swift resource directory, existed, but around 100 tests from the compiler validation suite failed with that, as they pass in a separate -sdk that simply has a Swift resource directory with a few different API notes or something, but does not contain the platform-specific files needed in <sdkRoot>/usr/lib/swift/<os>/. Actually checking for this OS-specific directory makes sure the Swift resource directory in -sdk contains the required platform-specific modules and libraries, and falls back to the Swift resource directory in the toolchain itself otherwise. However, that implies mixing and matching files from two different Swift resource directories, so maybe it will be better to enforce that only one Swift resource directory is used and modify all those tests instead?
For an example of such mixing already taking place, the Windows CI failed with this pull, right before trying to compile the trunk Foundation macros for the Windows host:
-- Check for working Swift compiler: T:/5/bin/swiftc.exe - broken
CMake Error at C:/Program Files/CMake/share/cmake-3.29/Modules/CMakeTestSwiftCompiler.cmake:40 (message):
The Swift compiler
"T:/5/bin/swiftc.exe"
is not able to compile a simple test program.
It fails with the following output:
Change Dir: 'T:/x86_64-unknown-windows-msvc/FoundationMacros/CMakeFiles/CMakeScratch/TryCompile-6u6j6e'
Run Build Command(s): C:/PROGRA~1/MICROS~2/2022/COMMUN~1/Common7/IDE/COMMON~1/MICROS~1/CMake/Ninja/ninja.exe -v cmTC_bc87d
[1/2][ 50%][0.084s] T:\5\bin\swiftc.exe -target x86_64-unknown-windows-msvc -j 36 -num-threads 36 -c -module-name cmTC_bc87d -sdk T:/toolchains/swift-6.0.3-RELEASE-windows10/LocalApp/Programs/Swift/Platforms/6.0.3/Windows.platform/Developer/SDKs/Windows.sdk -gnone -Xlinker /INCREMENTAL:NO -Xlinker /OPT:REF -Xlinker /OPT:ICF -incremental -output-file-map CMakeFiles\cmTC_bc87d.dir\\output-file-map.json T:\x86_64-unknown-windows-msvc\FoundationMacros\CMakeFiles\CMakeScratch\TryCompile-6u6j6e\main.swift
FAILED: CMakeFiles/cmTC_bc87d.dir/main.swift.obj
T:\5\bin\swiftc.exe -target x86_64-unknown-windows-msvc -j 36 -num-threads 36 -c -module-name cmTC_bc87d -sdk T:/toolchains/swift-6.0.3-RELEASE-windows10/LocalApp/Programs/Swift/Platforms/6.0.3/Windows.platform/Developer/SDKs/Windows.sdk -gnone -Xlinker /INCREMENTAL:NO -Xlinker /OPT:REF -Xlinker /OPT:ICF -incremental -output-file-map CMakeFiles\cmTC_bc87d.dir\\output-file-map.json T:\x86_64-unknown-windows-msvc\FoundationMacros\CMakeFiles\CMakeScratch\TryCompile-6u6j6e\main.swift
<unknown>:0: warning: using (deprecated) legacy driver, Swift installation does not contain swift-driver at: 'C:\Users\swift-ci\jenkins\workspace\swift-PR-windows\build\5\bin\swift-driver-new.exe'
<unknown>:0: warning: option '-incremental' is only supported in swift-driver
<unknown>:0: error: module compiled with Swift 6.0.3 cannot be imported by the Swift 6.2 compiler: T:/toolchains/swift-6.0.3-RELEASE-windows10/LocalApp/Programs/Swift/Platforms/6.0.3/Windows.platform/Developer/SDKs/Windows.sdk\usr\lib\swift\windows\Swift.swiftmodule\x86_64-unknown-windows-msvc.swiftmodule
ninja: build stopped: subcommand failed.
The issue appears to be that they're trying to build the trunk Foundation macros with the freshly-built trunk 6.2 Swift compiler, but passing in a Swift 6.0.3 SDK with -sdk T:/toolchains/swift-6.0.3-RELEASE-windows10/LocalApp/Programs/Swift/Platforms/6.0.3/Windows.platform/Developer/SDKs/Windows.sdk to build it. The trunk Swift 6.2 compiler then quietly ignores that 6.0.3 Swift resource directory and likely uses the 6.2 Swift resource directory next to the toolchain instead. This pull, by enforcing that the Swift resource directory in the 6.0.3 -sdk that is passed in is used because <6.0.3-sdkRoot>/usr/lib/swift/windows/ exists, then fails because the 6.0.3 modules cannot be used with the trunk Swift 6.2 compiler.
@weliveindetail, do you know if that's merely a configuration mistake when building the Foundation and Testing macros on the Windows CI, which should be easily fixed, or something more complicated?
| @@ -2258,6 +2259,18 @@ static bool ParseSearchPathArgs(SearchPathOptions &Opts, ArgList &Args, | |||
|
|
|||
| if (const Arg *A = Args.getLastArg(OPT_resource_dir)) | |||
| Opts.RuntimeResourcePath = A->getValue(); | |||
| else if (!Triple.isOSDarwin() && Args.hasArg(OPT_sdk)) { | |||
There was a problem hiding this comment.
This code explicitly follows what the original C++ Driver has long done when looking for the Swift runtime libraries, swiftrt.o, and a few other files found in the Swift resource directory:
if (const Arg *A = args.getLastArg(options::OPT_resource_dir)) {
StringRef value = A->getValue();
resourceDirPath.append(value.begin(), value.end());
} else if (!getTriple().isOSDarwin() && args.hasArg(options::OPT_sdk)) {
StringRef value = args.getLastArg(options::OPT_sdk)->getValue();
resourceDirPath.append(value.begin(), value.end());
llvm::sys::path::append(resourceDirPath, "usr");
CompilerInvocation::appendSwiftLibDir(resourceDirPath, shared);
} else {
auto programPath = getDriver().getSwiftProgramPath();
CompilerInvocation::computeRuntimeResourcePathFromExecutablePath(
programPath, shared, resourceDirPath);
}
Note how the SDK is only looked in if a non-Darwin -sdk is explicitly specified: Saleem later tried to expand that to Darwin also in #26361, but he may have never got it to work.
That C++ Driver setup now matches this Frontend setup, because the default in both is now to look relative to the compiler, which is done first in this Frontend here, ie usr/bin/../lib/swift/. If a -resource-dir is set, that is given first priority, then a non-Darwin -sdk is given second priority, ie the C++ Driver and the Frontend now match in where they look.
This is important for two reasons:
- The new swift-driver simply queries the Frontend and uses whatever Swift resource directory it uses, so now the new swift-driver finally matches the original C++ Driver's behavior, and piecemeal workarounds like that in [Unix] Go back to only checking the runtime resource path for swiftrt.o swift-driver#1822 can now be eliminated.
- The Frontend will now look in the same Swift resource directory for stdlib/corelibs swiftmodules as the swift-driver is looking for runtime libraries and
swiftrt.o, eliminating any confusion between the two by centralizing the Swift resource directory lookup here. That already found one bug in the Windows CI, see my other code comment.
However, unlike the C++ Driver, my -sdk code below actually checks if the -sdk path contains a Swift resource directory for the platform triple and does not use the -sdk for this if not, falling back to the aforementioned default next to the compiler in that case. This is because an -sdk is not guaranteed to have a Swift resource directory and may have only a C/C++ sysroot.
We should probably tighten this up to require an explicit -sdk to have a Swift resource directory, with the only exception when an explicit -resource-dir is also specified, but I'm open to debate here. The C++ Driver doesn't even check if the non-Darwin -sdk has a Swift resource directory and simply assumes one is there, we can do a bit better than that.
|
Alright, this tiny pull and the resulting swiftlang/swift-driver#1822 cleanup now pass the linux CI, have no effect on Darwin, and found a seeming bug in the Windows CI. I'm going to test this more in comparison to the C++ Driver and examine more tests to see if I can use it to flush out more cross-compilation bugs with an explicit non-Darwin In the meantime, the basic functionality works and is ready for review. I will add tests once the few remaining smaller design choices mentioned above are hammered out. @artemcm, @etcwilde, and @compnerd, please take a look: I'm looking for feedback on both this current patch and the remaining issues and questions in my detailed comments above. |
|
swiftlang/swift-driver#1827 |
|
swiftlang/swift-driver#1827 |
finagolfin
requested review from
zoecarver,
hyp,
egorzhdan,
beccadax,
ian-twilightcoder,
Xazax-hun,
j-hui and
fahadnayyar
as code owners
finagolfin
removed request for
hyp,
Xazax-hun,
egorzhdan,
zoecarver,
ian-twilightcoder and
fahadnayyar
This is a known issue, the libraries are moved to the arch-specific folder here. However, this is done after
I believe this change here is at fault. This has the side effect of removing the library include path from the linker invocation. I have a couple of pending changes [1][2] to install these libraries in the right folder. In the meantime, I believe that you are going to need to keep the above line for Windows for the time being. [1] swiftlang/swift-foundation#1463 |
Thanks, I figured it was being adjusted elsewhere.
Yep, that's intentional, as I explained in that pull, because after this frontend pull, the Testing build got confused between the trunk SDK headers and the Foundation build directory headers.
If you're planning to move this library anyway, it should work then: any reason you expect those move pulls to get delayed? I think it's best to switch Testing over to build only from the fresh trunk Windows SDK with this pull, and make whatever other changes are needed to make removing that line work. |
|
swiftlang/swift-corelibs-libdispatch#892 @swift-ci smoke test windows |
|
Finally got the Windows x86_64 SDK to build, with no regression on the passing linux CI either, but then building libdispatch in the next Windows i686 build fails because it assumes an empty SDK like the first one. Looks like the Windows CI is doing a more advanced build by populating an SDK from the beginning, but because this Instead, if I can get the Windows CI building with this I'll put in some hacks now just to get the Windows CI fully building, then try to clean all this up with proper pulls at the end. |
|
Make sure nothing regressed. swiftlang/swift-corelibs-libdispatch#892 @swift-ci test |
|
A single macOS test is failing, so dump the compiler output there and compare it to linux, where the same test passes. Put in a hack to move the Windows CI forward, where it checks if the first Windows SDK architecture has already been built and installed, then skips an unnecessary libdispatch flag for the second Windows arch onwards. swiftlang/swift-corelibs-libdispatch#892 @swift-ci smoke test |
|
swiftlang/swift-corelibs-libdispatch#892 @swift-ci smoke test windows |
|
swiftlang/swift-corelibs-libdispatch#892 @swift-ci smoke test windows |
|
swiftlang/swift-corelibs-libdispatch#892 @swift-ci smoke test windows |
… non-Darwin platform runtime libraries and modules too as done originally in swiftlang#25990 with the legacy C++ Driver, but since lost in the new swift-driver. Only difference is this checks if a Swift resource directory exists in `-sdk` and falls back to the default if not.
1 similar comment
as done originally in #25990 with the legacy C++ Driver, but since lost in the new swift-driver
This is another attempt to fix swiftlang/swift-driver#1562 and avoid the various workarounds that have been put in elsewhere to spackle over this root cause.
This worked for me with the compiler validation suite running natively on an Android device, with the exception of one SourceKit test. Let's run it through all the platforms on the CI before explaining fully what it's doing and adding a test to make sure it doesn't regress.